Don't restart servers when userprefs change #2448

jwortmann · 2024-04-15T14:42:04Z

Currently, all language servers are restarted whenever the user settings in LSP.sublime-settings are saved. This is not really necessary, so with this PR they are only restarted if the "clients" setting (containing the server configs) is actually modified.

Note that not all UI features are immediately redrawn when the user settings change. For example if you change "diagnostics_gutter_marker", it still uses the old icons until new diagnostics arrive (usually after next buffer modification). That is because the icon is stored directly in the DiagrosticSeverityData object

LSP/plugin/core/views.py

Lines 68 to 78 in 5891ff4

    
           class DiagnosticSeverityData: 
        
               __slots__ = ('regions', 'regions_with_tag', 'annotations', 'scope', 'icon') 
        
               def __init__(self, severity: int) -> None: 
        
                   self.regions = []  # type: List[sublime.Region] 
        
                   self.regions_with_tag = {}  # type: Dict[int, List[sublime.Region]] 
        
                   self.annotations = []  # type: List[str] 
        
                   _, _, self.scope, self.icon, _, _ = DIAGNOSTIC_SEVERITY[severity - 1] 
        
                   if userprefs().diagnostics_gutter_marker != "sign": 
        
                       self.icon = "" if severity == DiagnosticSeverity.Hint else userprefs().diagnostics_gutter_marker

which was maybe not the optimal design choice, but I didn't want to do huge refactorings here.

Or for example if inlay hints get enabled/disabled, they are only added or cleared after the next buffer modification, but I think that should be okay.

predragnikolic · 2024-04-16T13:22:12Z

I have not tested this,
but based on the code that I see

Previously,
if someone modified server settings,
because LSP restarts the server on setting change,
the server would pick it up the new changes once started again.

Now in this PR,
LSP will not restart the server, when we change server settings,
thus the server will not pick up new setting changes.

In order to address this,
the server would now need to be notified with didChangeConfiguration.

related to
#2044

jwortmann · 2024-04-16T14:32:27Z

The server settings are part of the "clients" setting, so if they are modified, servers are still restarted.

didChangeConfiguration would need to be implemented separately.

This PR is only for the userprefs, i.e. UI settings and other LSP settings. It is not directly related to #2044.

plugin/session_view.py

plugin/core/windows.py

plugin/core/settings.py

plugin/core/windows.py

plugin/session_buffer.py

rchl · 2024-09-24T18:43:35Z

Or for example if inlay hints get enabled/disabled, they are only added or cleared after the next buffer modification, but I think that should be okay.

Honestly, I'm not sure whether that's OK. That means that you touch some setting and now all the open views start missing something (semantic highlighting, for example). Now you have to either poke all the views or restart ST which kinda defeats the point of this fix. I think we would have to do the right thing and update every functionality that might be affected.

jwortmann · 2024-09-30T13:15:30Z

That means that you touch some setting and now all the open views start missing something (semantic highlighting, for example).

Semantic highlighting does get redrawn (added/removed) after changing the corresponding setting value. I think it should only be the inlay hints which are not updated yet (unless there is another feature that I missed); mostly because the phantom handling for inlay hints can be a bit complicated and I didn't write that code, so I haven't analyzed how exactly it works.

Now you have to either poke all the views or restart ST which kinda defeats the point of this fix. I think we would have to do the right thing and update every functionality that might be affected.

Well instead of restart ST you could restart only the language server from the command palette if you want to redraw inlay hints. But I can try to figure out what would need to be done to redraw inlay hints too.

jwortmann · 2024-10-01T12:52:07Z

Actually for inlay hints we were already doing almost the right thing. The "show_inlay_hints" setting is different in the way that it is only used as the default value for new windows. But we have a command and a menu item to toggle the inlay hints dynamically per window, so I assume the states for all windows that are already opened should not be affected when the global LSP setting is changed.

I think there was a small inconsistency with the settings description in the LspToggleInlayHintsCommand that it would only set the initial state when it was toggled the first time. But this should be fixed now.

In contrast to what I wrote above, I saw that semantic highlighting is currently only removed when the setting is turned off, but not added when turned on. I'll take another look later how it can be redrawn directly.

jwortmann · 2024-10-02T16:16:43Z

Semantic tokens should now be requested/updated if the userprefs change and semantic highlighting is enabled (regardless whether that setting actually changed or not), as soon as the view gets activated. I think this is a good compromise between having the rendering state up to date and not overwhelming the server with many expensive requests.

* main: style change Don't restart servers when userprefs change (#2448)

Don't restart servers when userprefs changes

18db9d0

jwortmann added 2 commits April 19, 2024 19:44

Merge branch 'main' into no-restart

b27b317

Merge branch 'main' into no-restart

8a16356

jwortmann mentioned this pull request Apr 22, 2024

Feature: Only format when project configured for formatting #2412

Open

jwortmann added 3 commits April 25, 2024 11:04

Merge branch 'main' into no-restart

aaaee94

Resolve more merge conflicts

4f57303

Merge branch 'main' into no-restart

8d0daff

jwortmann mentioned this pull request Jul 23, 2024

Feature request: Toggle showing of diagnostics #2161

Open